Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DOCS#2082] Documents configurations for CNI policy-setup timeouts #1373

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

aaaaaaaalex
Copy link
Contributor

@aaaaaaaalex aaaaaaaalex commented Mar 13, 2024

Product Version(s):
OSS 3.28 (EE 3.19 EP2 to come at a later date)

Issue:
https://tigera.atlassian.net/browse/DOCS-2082

Link to docs preview:
https://deploy-preview-1373--calico-docs-preview-next.netlify.app/calico/next/reference/configure-cni-plugins#enabling-policy-setup-timeout

https://deploy-preview-1373--calico-docs-preview-next.netlify.app/calico/next/reference/felix/configuration (See manifest section -> EndpointStatusPathPrefix field)

https://deploy-preview-1373--calico-docs-preview-next.netlify.app/calico/next/reference/resources/felixconfig similar to previous link

SME review:

  • An SME has approved this change.

DOCS review:

  • A member of the docs team has approved this change.

Additional information:

Merge checklist:

  • Deploy preview inspected wherever changes were made
  • Build completed successfully
  • Test have passed

@aaaaaaaalex aaaaaaaalex requested a review from a team as a code owner March 13, 2024 15:21
Copy link

netlify bot commented Mar 13, 2024

Deploy Preview succeeded!

Built without sensitive environment variables

Name Link
🔨 Latest commit eca5361
🔍 Latest deploy log https://app.netlify.com/sites/tigera/deploys/65f4384e12bfeb00081b125c
😎 Deploy Preview https://deploy-preview-1373--tigera.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 73 (🟢 up 24 from production)
Accessibility: 93 (no change from production)
Best Practices: 83 (no change from production)
SEO: 86 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Mar 13, 2024

Deploy Preview for calico-docs-preview-next ready!

Name Link
🔨 Latest commit eca5361
🔍 Latest deploy log https://app.netlify.com/sites/calico-docs-preview-next/deploys/65f4384eb914c500082add92
😎 Deploy Preview https://deploy-preview-1373--calico-docs-preview-next.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 55 (🔴 down 11 from production)
Accessibility: 93 (no change from production)
Best Practices: 92 (no change from production)
SEO: 79 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Collaborator

@ctauchen ctauchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this @aaaaaaaalex. A few comments and suggestions for you.

@@ -248,6 +248,35 @@ you must also run calico/kube-controllers with the policy, profile, and workload

When using `type: k8s`, the {{prodname}} CNI plugin requires read-only Kubernetes API access to the `Pods` resource in all namespaces.

### Enabling Policy Setup Timeout

If you wish for the CNI to delay a new pod from starting until the pod's policy has been programmed to the dataplane,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can ignore this Vale error for now.

Comment on lines 268 to 272
:::note

A `policy_setup_timeout_seconds` of 0 disables any policy-setup waiting.

:::
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what this note adds that isn't clear from the idea of setting it to 0 seconds. Am I missing something? Can we get rid of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not missing anything, over verbose on my part 😄

Comment on lines 274 to 278
:::note

If the pod's policy is not programmed after the number of seconds specified, the CNI will allow the pod to start its containers anyway.

:::
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move this to the intro text, without the note formatting. (I've added to comment above.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think with different phrasing of the intro text, this idea is clear. Let me know what you think.

@@ -248,6 +248,35 @@ you must also run calico/kube-controllers with the policy, profile, and workload

When using `type: k8s`, the {{prodname}} CNI plugin requires read-only Kubernetes API access to the `Pods` resource in all namespaces.

### Enabling Policy Setup Timeout
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### Enabling Policy Setup Timeout
### Enabling policy setup timeout
  • We use sentence case for title and headings.

I'm not 100% clear on the distinction in this document between generic and kubernetes-specific configuration. Just want to check this is in the right place. Is the idea that this doesn't apply, for example, to an OpenStack deployment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct - this functionality lives in the k8s portion of our CNI 👍

@@ -248,6 +248,35 @@ you must also run calico/kube-controllers with the policy, profile, and workload

When using `type: k8s`, the {{prodname}} CNI plugin requires read-only Kubernetes API access to the `Pods` resource in all namespaces.

### Enabling Policy Setup Timeout

If you wish for the CNI to delay a new pod from starting until the pod's policy has been programmed to the dataplane,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some new copy, based partly on the previous sections and incorporating the idea of a note you have below:

The `policy_setup_timeout_seconds` option makes the CNI plugin wait to start a new pod until one of the following conditions occurs:

* The pod's policy has finished being programmed.
* A specified amount of time has elapsed.

By setting this option, you can avoid errors that can occur when a pod tries to start before the pod's policy is programmed.

Example CNI config:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brilliant, thanks!

@ctauchen
Copy link
Collaborator

LGTM! Ready to merge?

@aaaaaaaalex
Copy link
Contributor Author

Thanks @ctauchen , yes I think we're good to merge 🚀

@ctauchen ctauchen merged commit 0ee5f63 into tigera:main Mar 15, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants